Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: implement export page #586

Merged
merged 2 commits into from
Sep 14, 2023
Merged

feat: implement export page #586

merged 2 commits into from
Sep 14, 2023

Conversation

kyrylo-kh
Copy link
Contributor

  • Implemented new page - Export page
  • Created new generic component - CourseStepper (also will be used in import page)

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Sep 5, 2023
@openedx-webhooks
Copy link

Thanks for the pull request, @Thposbsmt! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@mphilbrick211 mphilbrick211 added the needs test run Author's first PR to this repository, awaiting test authorization from Axim label Sep 5, 2023
@codecov
Copy link

codecov bot commented Sep 5, 2023 •
edited
Loading

Codecov Report

Patch coverage: 95.95% and project coverage change: +0.35% 🎉

Comparison is base (a1793ef) 84.55% compared to head (74a9a37) 84.91%.
Report is 8 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #586      +/-   ##
==========================================
+ Coverage   84.55%   84.91%   +0.35%     
==========================================
  Files         307      330      +23     
  Lines        4993     5296     +303     
  Branches     1102     1165      +63     
==========================================
+ Hits         4222     4497     +275     
- Misses        747      753       +6     
- Partials       24       46      +22
Files Changed Coverage Δ
src/CourseAuthoringRoutes.jsx 100.00% <ø> (ø)
src/store.js 100.00% <ø> (ø)
src/export-page/data/thunks.js 82.85% <82.85%> (ø)
src/export-page/export-stepper/ExportStepper.jsx 96.00% <96.00%> (ø)
src/export-page/CourseExportPage.jsx 96.15% <96.15%> (ø)
src/export-page/data/api.js 100.00% <100.00%> (ø)
src/export-page/data/constants.js 100.00% <100.00%> (ø)
src/export-page/data/selectors.js 100.00% <100.00%> (ø)
src/export-page/data/slice.js 100.00% <100.00%> (ø)
src/export-page/export-footer/ExportFooter.jsx 100.00% <100.00%> (ø)
... and 10 more

... and 44 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@e0d
Copy link

e0d commented Sep 5, 2023

Hi @Thposbsmt it looks like this PR reduces coverage, can you have a look?

@e0d e0d added waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. and removed needs test run Author's first PR to this repository, awaiting test authorization from Axim labels Sep 5, 2023
@kyrylo-kh kyrylo-kh force-pushed the kholodenko/export-page branch 4 times, most recently from 51f9e0b to 3050dfb Compare September 11, 2023 09:44
KristinAoki
KristinAoki reviewed Sep 11, 2023
View reviewed changes
src/export-page/messages.js Outdated Show resolved Hide resolved
src/export-page/data/api.js Outdated Show resolved Hide resolved
src/export-page/data/thunks.js Outdated Show resolved Hide resolved
src/export-page/export-modal-error/ExportModalError.jsx
courseId,
}) => {
const dispatch = useDispatch();
const isErrorModalOpen = useSelector(getIsErrorModalOpen);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What was the reasoning for use redux to control the state of the visibility of the error modal versus useToggle from paragon

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redux allow to change state anywhere. In this case isErrorModalOpen changes in thunks and ExportModalError.jsx. We cannot do it with useToggle

@KristinAoki KristinAoki merged commit 1888993 into openedx:master Sep 14, 2023
5 checks passed
@sundasnoreen12 sundasnoreen12 mentioned this pull request Oct 25, 2023
Merged
@sundasnoreen12
Copy link
Contributor

There is a UI glitch on the MFE because of adding custom css for this paragon class "pgn__stepper-header-step-list ".
I added the fix in this PR. Please review again at your end and let me know is the provided fix is also working fine at your end.
@KristinAoki

@openedx-webhooks
Copy link

@kyrylo-kh 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

snglth pushed a commit to Abstract-Tech/community-theme-course-authoring that referenced this pull request Jan 9, 2024
@kyrylo-kh @snglth
feat: implement export page (openedx#586)
5885476
@kenclary kenclary mentioned this pull request Jan 9, 2024
Merged
@Lunyachek Lunyachek mentioned this pull request Mar 5, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KristinAoki KristinAoki KristinAoki approved these changes

Assignees
No one assigned
Labels
open-source-contribution PR author is not from Axim or 2U waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc.
Projects
Contributions
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@kyrylo-kh @openedx-webhooks @e0d @sundasnoreen12 @KristinAoki @mphilbrick211